[kesten_processes] Kesten Processes lecture update#591
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
📖 Netlify Preview Ready! Preview URL: https://pr-591--sunny-cactus-210e3e.netlify.app (5b0ce92) 📚 Changed Lecture Pages: kesten_processes |
HumphreyYang
left a comment
There was a problem hiding this comment.
Many thanks @kp992, it looks really nice!
Just some minor comments!
lectures/kesten_processes.md
Outdated
| # Use vmap to parallelize over the M dimension | ||
| vectorized_single_draw = vmap( | ||
| generate_single_draw, | ||
| in_axes=(0, None, None, None, None, None, None, None, None, None), | ||
| ) |
There was a problem hiding this comment.
I think one trick to avoid many Nones is by multiplying list and then unwrap:
vectorized_single_draw = vmap(
generate_single_draw,
in_axes=(0, *[None]*9),
)There was a problem hiding this comment.
Why are there so many None needed here? Seems like a crazy function interface choice (coming from someone who has never used vmap before.
There was a problem hiding this comment.
I think they want to take into account mapping across all possible dimensions and output of all possible shapes so they ended up having this massive choice set.
There was a problem hiding this comment.
Okay, I will change this a bit to keep less None
|
@kp992 @HumphreyYang I haven't come across this error before I don't see this happening in other PRs -- but looks like an install issue? It is coming from @jit
def generate_draws(
key=random.PRNGKey(123),
μ_a=-0.5,
σ_a=0.1,
μ_b=0.0,
σ_b=0.5,
μ_e=0.0,
σ_e=0.5,
s_bar=1.0,
T=500,
M=1_000_000,
s_init=1.0,
):
"""
JAX-jit version of the generate_draws function.
Returns:
Array of M draws
"""
# Create M different random keys for parallel execution
keys = random.split(key, M)
draws = vectorized_single_draw(
keys, μ_a, σ_a, μ_b, σ_b, μ_e, σ_e, s_bar, T, s_init
)
return draws |
|
Hi @mmcky, The code runs well on my GPU, I think this might be related to the environment. I will retrigger the action and see if it helps. |
|
📖 Netlify Preview Ready! Preview URL: https://pr-591--sunny-cactus-210e3e.netlify.app (6e44c3c) 📚 Changed Lecture Pages: kesten_processes |
|
Hi @mmcky, This is fixed now. It's a bit tricky to find but it is caused by a minor typo |
Thanks for fixing this. Even for me it worked fine. Probably we can work on making debug messages easier. |
Many thanks @kp992, I was expecting bug like this to be raised by |
The |
|
thanks @kp992 you read my mind! Was just about to make the edit :) Will merge this soon. Thanks for your PR. |
|
@kp992 once this finishes building -- can you please do a final output check of the preview link and give me your thumbs up. |
|
📖 Netlify Preview Ready! Preview URL: https://pr-591--sunny-cactus-210e3e.netlify.app (1adfc93) 📚 Changed Lecture Pages: kesten_processes |
|
Looks good to me @mmcky |
|
📖 Netlify Preview Ready! Preview URL: https://pr-591--sunny-cactus-210e3e.netlify.app (9bead7e) 📚 Changed Lecture Pages: kesten_processes |
|
@mmcky I'm not seeing any JAX on the 'Changed lecture pages' link above. |
|
We have imports in the
We typically have a rule that all imports are at the top of the lecture, however I am in two minds re: solutions. Half of me thinks keeping imports allowed in solutions can be useful when introducing a new package that hasn't been used in the lecture itself. Any thoughts on this |
|
thanks @HumphreyYang I have started on the documentation in the I will use this one as a final cross check and then do a final edit of https://github.com/QuantEcon/QuantEcon.manual/pull/73/files. |
|
📖 Netlify Preview Ready! Preview URL: https://pr-591--sunny-cactus-210e3e.netlify.app (7d00832) 📚 Changed Lecture Pages: kesten_processes |
|
thanks @HumphreyYang for these changes. @jstac -- I am in favour of consistency, but this change seems to be adding quite a few lines of code (making it more complex) -- so I just wanted to run this template by you to make sure we are heading down the right path here. |
|
@mmcky This is, in a sense, the cost of using JAX. JAX wasn't built for iteration and has no immutable arrays. If we're going to keep using the lax.scan logic, then, given it's slightly unusual syntax, I think it's better to pull that logic out into a specialized function and explain it. I recommend not hiding this code, since it's essential to the lecture. I'm very open to other ideas and approaches. The other option is refrain from JAX conversions for lectures that iterate a low dimensional object forward many times. Admittedly, numpy and numba are better for this. I'm open to this discussion too. |
|
Thanks @jstac.
Completely agree.
👍
I guess I would like to have
|
Are you thinking of something like I will mull over this tonight. |
|
@HumphreyYang you've done a great job with this conversion. It is going to be a really useful pattern to finalise the docs. |
This reverts commit 7d00832.
|
📖 Netlify Preview Ready! Preview URL: https://pr-591--sunny-cactus-210e3e.netlify.app (4f4c0f5) 📚 Changed Lecture Pages: kesten_processes |


This PR: